feat(border): [android] border-style: none support
#47212
Conversation
facebook-github-bot
added
CLA Signed
NickGerleman
left a comment
NickGerleman
left a comment
There was a problem hiding this comment.
This causes border not to render, which we want, but doesn't account for border widths still being passed to Yoga and consuming layout space (compared to web where it acts as if there is no border present).
Oh, that's quite an interesting point. Will look into that, thanks for the very quick feedback! @NickGerleman |
…rt-fixes feat(styles): setting border style in yoga and applying logic to for `none`
|
@NickGerleman I've updated my approach to account for the widths that were still being passed to Yoga and consuming layout space. There are still a few things I need to tweak in this PR, but before putting more effort into that, I would appreciate some feedback on whether this approach is heading in the right direction. I've also updated my PR description and examples to demonstrate that it is working correctly on Android. |
NickGerleman
left a comment
NickGerleman
left a comment
There was a problem hiding this comment.
Adding BorderStyle to Yoga would unfortunately be more involved than what is in this PR. Some of the code here is normally generated, in the Yoga repo (see the copied "generated from enums.py"), and the edits here are currently to private boundary in Yoga we are wanting to move things off of, instead of public API. We also want new code in Yoga to have unit tests (which can be added to OSS repo).
We also don't want to add APIs to Yoga which don't do anything (like, the layout engine doesn't care about whether a border is dotted or solid. It just needs to know the insets the border will take).
I think this would need to be organized as deriving border widths set on Yoga node, from the borderStyle (to know if borders should be ignored). Though we should also be careful not to add too much complexity here, if this change ends up being very involved.
|
Abandoning this PR as could not allocate time to find a simpler way to derive the border |
3 participants
Summary:
As part of #34425,
borderStylevalue ofnoneshould be supported.In this PR, the
BorderStyleenum is extended and the logic is implemented to make thenonevalue work in bothBorderDrawableandCSSBackgroundDrawableas currently there is a feature flagenableNewBackgroundAndBorderDrawablescontrolling which class draws the border. The logic has been implemented in both classes so it works regardless of the feature flag value.Also, a new style property (
borderStyle) has been implemented in the Yoga layer to be able to decide whether to pass layout spacing from the border. Now, if the border style is set tonone, no layout space will be consumed.In this PR the border style value doesn't explicitly expose the newly accepted property
nonein the types yet as the implementation for iOS is still needed. Notice theFlowFixMein the examples.Changelog:
[ANDROID][ADDED] -
border-style: nonesupportTest Plan:
Two new examples are added. One for
Textand one forView– both of them are setting some properties for the border which by default makes the components draw a border with a defaultBorderStyle.SOLID, but since the propertyborderStyle: 'none'is also passed, that default is overwritten hence you see no border applied.In the screenshots, the border widths are set to
10pxto be able to test that when the border style is set tonone, it accounts for the widths not consuming layout space.Both examples have been tested with the
enableNewBackgroundAndBorderDrawablesfeature flag on and off.